✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.86%. Comparing base (497eaf4) to head (0b23c70).
Report is 141 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3097      +/-   ##
==========================================
+ Coverage   87.12%   87.86%   +0.74%     
==========================================
  Files         200      195       -5     
  Lines        6109     5986     -123     
==========================================
- Hits         5322     5259      -63     
+ Misses        787      727      -60     

I'm a bit puzzled—why are we reintroducing support for building with GCC 4.8 and C++11 when we've already moved on to supporting C++14 and above? Or may be I am missing something :)

Instead of reintroducing nostd helpers for code in the sdk already using std::unique_ptr, I think it will be better to settle on a subset of C++ 14.

Let's call itC++13.9 ... almost as C++14, but we restrict ourselves to not use std::make_unique, so that it can also build in C++11.

There are 4 lines to fix in the entire code base, so that:

data_->Add(std::make_unique<ResourceMetrics>(data));

needs to change to

data_->Add(std::unique_ptr<ResourceMetrics>(new ResourceMetrics(data)));

and then very old toolchains will still work.

While C++11 is no longer officially supported, we should not break it if it is easy (4 lines) to avoid.

@owent would this work for you ?

Previous discussion on C++11 / C++14:

#2522 (comment)

Yes, prefer not introducing nostd::make_unique as the C++ 14 is already the minimum requirement. And we also don't want to declare that it is compatible with std::make_unique in C++ 14 and above, like nostd::make_unique(nostd::size_t size) is not supported. So could outline the current code like @marcalff mentioned, or give it some other utility name if it is really necessary. This way makes it more flexible than polyfill a standard library method.

Instead of reintroducing nostd helpers for code in the sdk already using std::unique_ptr, I think it will be better to settle on a subset of C++ 14.

Let's call itC++13.9 ... almost as C++14, but we restrict ourselves to not use std::make_unique, so that it can also build in C++11.

There are 4 lines to fix in the entire code base, so that:

data_->Add(std::make_unique<ResourceMetrics>(data));

needs to change to

data_->Add(std::unique_ptr<ResourceMetrics>(new ResourceMetrics(data)));

and then very old toolchains will still work.

While C++11 is no longer officially supported, we should not break it if it is easy (4 lines) to avoid.

@owent would this work for you ?

Yes, it works. But I wonder if it's necessary to keep nostd::unique_ptr<T> if we always depend std::unique_ptr ?

I raised #3098 to replace std::make_unique only. And this PR can be closed if we use #3098 .

Yes, it works. But I wonder if it's necessary to keep nostd::unique_ptr<T> if we always depend std::unique_ptr ?

I raised #3098 to replace std::make_unique only. And this PR can be closed if we use #3098 .

In my understanding, nostd::unique_ptr<T> is now always implemented using std::unique_ptr internally (because we use C++ >= 11), but the prototype in the ABI only refers to nostd::unique_ptr<T>.

If we change the code in the API to replace nostd with std, the methods signatures will change, breaking the ABI.

Closed in favor of alternate solution, implemented in #3098